Skip to content

Code for activity model#7

Open
patricia-ternes wants to merge 2 commits intomainfrom
activitymodel
Open

Code for activity model#7
patricia-ternes wants to merge 2 commits intomainfrom
activitymodel

Conversation

@patricia-ternes
Copy link
Copy Markdown
Collaborator

PR overview

The folder 01-ActivityModel/activity-model/ contains the code to create an enriched synthetic population.

The instructions to install, configure and run the package are described in the README.md file.

The method data_preparation.Epc.remove_duplicates fix #5.

Reviewers

@nickmalleson and @mfbenitezp,

it would be wonderful if you could follow the instructions to download, install, configure and run the package, and give me any feedback on how the code works or about the instructions.

Thank you.

from enriching_population import EnrichingPopulation

if __name__ == "__main__":
print("hi")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have add some suggestion in the #9

Copy link
Copy Markdown
Collaborator

@mfbenitezp mfbenitezp Apr 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patricia-ternes @nickmalleson I have tested successfully the lastest code in this branch. I was able to run it for Leeds, and I have just a few comments that I have included in #9. Here are again my comments:

  1. I think the readme can also include some instructions on how o run the model., more like guide the user to going from one readme to another one. Or maybe the model can be wrapped in another folder structure.
  2. In the readme of /config, is there any way to run it to the whole local authorities?
  3. Once the required parameters are set, the rest of the text can be part of some section like extended or expand the model to the users needs.
  4. I replaced the hi print for another message.

@mfbenitezp mfbenitezp self-requested a review April 19, 2022 06:31
Copy link
Copy Markdown
Collaborator

@mfbenitezp mfbenitezp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. I could test it successfully and run it for Leeds, the results are included in the
#9


# EPC credentials
epc_user: "user@email"
epc_key: "user_key"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the readme I have included additional description, where users can understand epc_key means, the key_api https://epc.opendatacommunities.org/docs/api

area_url: "https://www.arcgis.com/sharing/rest/content/items/8a824519215947da99146692b0a0ff49/data"

area_in_out:
- "pcds"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pcds" # What does this represent?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to run the model for MSOA areas, should I remove the "oa11cd" and replace for the code of the msoa describes in the readme?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I update this file adding - "msoa11cd" as areas_in_out, but then I got this:

/Users/fbenitez/opt/anaconda3/envs/energyflex1/lib/python3.9/site-packages/causalinference/core/propensity.py:173: RuntimeWarning: invalid value encountered in sqrt
  return np.sqrt(np.diag(np.linalg.inv(H)))

And the PS isn't working
image

@nickmalleson
Copy link
Copy Markdown
Collaborator

Thanks for this @patricia-ternes . I think it'll take me a week to look at this, but as @mfbenitezp has already given really good comments I recommend merging now so you're not waiting for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EPC Duplicate Certificates

3 participants